-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sockets - Cyndi #29
base: master
Are you sure you want to change the base?
Sockets - Cyndi #29
Conversation
very clean looking! I think you missed the case for nil and empty array though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm Eric. I'll be your volunteer code reviewer. I've been a software developer for about a zillion years. I'll be applying most of the same professional standards to your code as I do in the day job. Some of my comments may seem pedantic, persnickety, or irrelevant to the kinds of exercises you're doing in class, but once you start your internship, you'll get the same kind of feedback I'm giving you here.
Good code for your first assignment.
@@ -0,0 +1,8 @@ | |||
def array_equals(arr_integers_1, array_integers_2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter names are inconsistent: arr_1 and array_2.
Parameter names are also unnecessarily verbose. array1 and array2 (as in the problem statement) would be fine.
@@ -0,0 +1,8 @@ | |||
def array_equals(arr_integers_1, array_integers_2) | |||
arr_integers_1.length.times do |index| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to handle the cases of array1 == nil, array2 == nil, array1.length != array2.length. This statement DOES cover zero-length arrays.
@@ -0,0 +1,8 @@ | |||
def array_equals(arr_integers_1, array_integers_2) | |||
arr_integers_1.length.times do |index| | |||
return false if arr_integers_1[index] - array_integers_2[index] != 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a != b is much clearer than a - b != 0, even if mathematically equivalent.
No description provided.